Add support for BNGL models (cont.) - #508
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #508 +/- ##
==========================================
+ Coverage 75.75% 76.01% +0.25%
==========================================
Files 65 67 +2
Lines 7357 7524 +167
Branches 1323 1341 +18
==========================================
+ Hits 5573 5719 +146
- Misses 1285 1302 +17
- Partials 499 503 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Add a BnglModel loader (a peer of PySBModel/SbmlModel) so that a `language: bngl` PEtab problem loads and validates via petablint / Problem.from_yaml at the model level. See PEtab-dev/PEtab#436. - petab/v1/models/bngl_model.py: BnglModel backed by a small, dependency-free BNGL block reader (parse_bngl). Introspection only; is_valid shells out to `BNG2.pl --check` when a BNG backend is locatable and falls back to True otherwise (mirroring how the SBML loader always validates because libsbml is always present). - Register `bngl` in known_model_types and add a branch to model_factory; v2 picks it up via the existing re-exports (+ a v2 shim module). - tests/v1/test_model_bngl.py and a minimal BNGL fixture: ABC unit tests plus a full Problem.from_yaml validation oracle covering the model-cross checks. # Conflicts: # petab/v1/models/__init__.py
Re-sync with PyBNF's sibling reader (pybnf/petab/_bngl.py, ADR-0026 / lanl/PyBNF#437): - Block aliases: `begin molecules` / `begin species` / `begin rules` now open the same blocks as `molecule types` / `seed species` / `reaction rules` (_block_lines consults a per-canonical-name alias table), per the BNGL grammar reference (BioNetGen Perl2/; BNG_vscode_extension docs/bngl-grammar.md). - Seed-species `$` clamp: `SeedSpeciesDefn = ["$"], Species, ...` -- the `$` fixed-concentration marker is stripped so `$counter() 10` enumerates the state variable `counter()`, keeping is_state_variable correct under the clamp. Adds grammar-hardening tests (alias parsing, `$`-clamp stripping, no cross-block shadowing, the is_state_variable seam) that double as the drift anchor against PyBNF's reader. ruff check + format clean; 19 passed. Refs: PEtab-dev/PEtab#436.
A trailing `\` (BNGL line continuation) splits one logical declaration across physical lines. The block scanner processed physical lines, so a continued parameter/function/observable was truncated at the `\` (e.g. `k = \` read as the value `\`). Add _logical_lines() mirroring BNG2.pl's readFile (Perl2/BNGModel.pm): strip the comment first, then while a line ends with `\` drop it and concatenate the next comment-stripped physical line directly (no space, so `1e\`+`3` -> `1e3`). Surfaced by the bng_parity corpus (895 community BNGL models): 252 use line continuation, incl. inside enumerated blocks (functions, observables, parameters, seed species). Adds continuation + backslash-in-comment tests; kept in sync with PyBNF's sibling reader (pybnf/petab/_bngl.py). ruff clean, 21 passed. Refs: PEtab-dev/PEtab#436.
BNGL declarations may carry a leading line label (LineLabel = {Digit}, WS |
Name, ":", [WS]): a legacy .net-style numeric index (`1 L0 1`) or a named label
(`CD14: CD14(...)`). The reader took the label as the entity -- the index as a
parameter name, the label as the seed species. Add _strip_line_label() and apply
it in the parameter and seed-species extractors (a valid BNGL identifier starts
with a letter, so a leading digit-run is unambiguously an index; a compartment
prefix carries `@`, so a bare `Name:` is unambiguously a label).
Surfaced by a writeModel-based differential over the bng_parity corpus (895
community models): 4 models disagreed with BNG2.pl's canonical parse (indexed
params/seed, labeled seed); after this fix, 0 -- parameters/observables/
functions/molecule-types/compartments all match BNG2.pl across the corpus.
Kept in sync with PyBNF's sibling reader. ruff clean, 24 passed.
Refs: PEtab-dev/PEtab#436.
Asserts parse_bngl enumerates the same model entities BNG2.pl does, over 21 curated public community BNGL models (RuleHub, BNGL-Models) under tests/v1/bngl_corpus/. BNG2.pl's answers are cached in golden.json -- the entity name sets it emits from `writeModel` (its canonical parse, no network generation) -- so the test needs NO BNG2.pl and runs anywhere; it compares the reader against the frozen oracle. Seed species are compared by molecule composition to absorb BNG2.pl's pattern canonicalization (t vs t(), component reordering, @compartment prefix vs suffix). The models exercise every hardened reader path: line continuations, indexed and labeled declarations, block aliases, the $ clamp, compartmental BNGL, energy patterns, states/bonds, component reordering, bare-molecule seed species. The golden is regenerated deliberately (needs BNG2.pl) via `python tests/v1/test_bngl_corpus.py` and reviewed as a diff. Mirrors PyBNF's live-BNG2.pl gate (lanl/PyBNF); validated there over the full 895-model bng_parity corpus (894/894 BNG2.pl-accepted models agree). ruff clean; 21 passed without BNG2.pl. Refs: PEtab-dev/PEtab#436.
…ies`)
The grammar doc lists `molecules` (for `molecule types`) and `rules` (for
`reaction rules`) as block aliases, but BNG2.pl 2.9.3 -- the reference this
reader targets -- REJECTS both ("Could not process block type 'molecules' /
'rules'"). Honoring them let the reader enumerate entities from a block BNG2.pl
refuses, i.e. accept models the reference rejects. Restrict _BLOCK_ALIASES to
`species` (for `seed species`), which BNG2.pl accepts and in fact emits as its
own canonical seed-species spelling. Verified empirically against BNG2.pl 2.9.3.
The corpus gate is unchanged (no fixture uses the dropped aliases; golden
regenerates identically). ruff clean; 24 passed. Refs: PEtab-dev/PEtab#436.
The 21 third-party .bngl fixtures under tests/v1/bngl_corpus/ are unmodified (or, for Barua_2009, one-line-patched) copies of files already published in RuleWorld/RuleHub and wshlavacek/BNGL-Models. Rather than vendoring ~2600 lines of someone else's model text, fetch the same 21 files on demand from their pinned upstream commits, sha256-verified against the exact bytes reviewed here. golden.json and README.md stay committed -- they're this repo's own oracle/test code, not sourced from anywhere upstream. See scripts/fetch_bngl_corpus_demo.py for the fetcher.
Add tests/v1/fetch_bngl_corpus.py: fetches the 21 .bngl fixtures backing tests/v1/test_bngl_corpus.py from their pinned upstream commits (RuleHub, BNGL-Models) via jsdelivr's GitHub CDN, sha256-verified against the exact bytes reviewed. No git/subprocess use, no execution of fetched content. Wire it into CI as a step before the unit tests run, gitignore the fetched .bngl files (golden.json/README.md stay tracked), and give test_reader_matches_bng2_golden a real skip reason pointing at the fetch script when the corpus hasn't been materialized yet.
d1124cf to
170f219
Compare
dilpath
left a comment
There was a problem hiding this comment.
Overall, fine for me since it doesn't affect much pre-existing code.
Will there be example PEtab problems with BNGL models here in the tests, or in the petab_test_suite?
Or some guide for BNGL users to describe how e.g. only species exported via BNGL observables are valid for PEtab observable formulae?
| _RULEHUB, | ||
| "Published/Barua2009/Barua_2009.bngl", | ||
| "26ca5053c4a340b597b2d839edd736469fc14cc2a03cf96b0447b7537089c454", | ||
| ("atoll=>", "atol=>", 1), |
There was a problem hiding this comment.
Should rather be an rulehub PR? Then remove the "repair" feature.
There was a problem hiding this comment.
I am not familiar with the policy there, but I assume it's deliberately untouched there.
| # Running this file directly makes the interpreter prepend its own directory | ||
| # to sys.path. The `math/` directory then shadows the stdlib `math` module, | ||
| # resulting in an ImportError. Drop that entry before importing anything that | ||
| # could pull in `math`. | ||
| _SCRIPT_DIR = str(Path(__file__).resolve().parent) | ||
| if sys.path and sys.path[0] == _SCRIPT_DIR: | ||
| del sys.path[0] |
There was a problem hiding this comment.
Clean up PATH after its modification is no longer needed, instead of here?
There was a problem hiding this comment.
I am not sure what you mean. This is not about the PATH environment variable. It's Python's import machinery and our directory names here.
|
There are PEtab lessons included in the PyBNF tutorial: |
Ideally yes, but I don't see that as a requirement for adding BNGL support here.
Some additional model-type specific information would be valuable, yes. However, I don't think that belongs here - I would add that to https://github.com/PEtab-dev/PEtab. |
…ged #508) (#590) The BnglModel loader PyBNF has been validating through a personal fork branch is now merged upstream (PEtab-dev/libpetab-python#508). Repoint the pytest-native-bngl leg's petab-spec from that fork branch to PEtab-dev/libpetab-python@main so the leg dogfoods the actual merged code (and survives the fork branch being deleted), and refresh the now-stale "Step B fork" wording across the workflow, the composite action, the _bngl drift note, and the pyproject comments. Runtime is unchanged. register_bngl() already collapses to a no-op against native support -- verified locally against upstream main: 'bngl' is a known model type, petab.v2.core imports, the no-op path triggers, and the native BnglModel parses parameters/observables. The latest PyPI petab (0.8.2) does not yet carry BNGL, so the petab>=0.8,<1 pin and the monkeypatch fallback both stay. Retiring them is a follow-up gated on the first PyPI release that ships BNGL.
…at petab 0.9.0 ships the loader, and drop Python 3.11 (#591) (#682) petab 0.9.0 (2026-09-07) is the first PyPI release whose known_model_types includes bngl: the native BnglModel loader PyBNF contributed upstream in PEtab-dev/libpetab-python#508 shipped. That is the trigger #591 was gated on. On any 0.9.0 installation register_bngl() already saw bngl registered and returned without rebinding, so the local adapter was dead at runtime and only the tests that constructed it directly still exercised it. Removed: pybnf/petab/bngl_model.py (the BnglModel adapter, register_bngl(), and its _locate_bng2 helper); the pytest-native-bngl CI leg that installed upstream libpetab-python main to reach the native loader ahead of a release; the setup-pybnf action's petab-spec input that leg used; the automodule entry for the removed module; and every register_bngl() call in the tests, the tutorial lessons (12, 13), the lint-clinic fixture generator, and the three real-world make_petab.py scripts. The petab and tests extras now floor at petab >= 0.9, and the action installs the same pin, which test_packaging_metadata already cross-checks. petab 0.9.0 requires Python >= 3.12, so the project floor moves to 3.12: requires-python, the classifier, ruff's target-version, the CI matrix, the packaging test, and the install/cluster docs and batch scripts that named 3.11. Kept: pybnf/petab/_bngl.py, the reader the importer and exporter use (its drift note now points at the shipped upstream module), and pybnf/petab/_bngl_expr.py, the parameter-expression evaluator from #666. The adapter was the evaluator's only consumer, and petab's native BnglModel does not evaluate an expression-valued parameter (get_parameter_value raises NotImplementedError, get_free_parameter_ids_with_values skips it). That is not a regression of this change, since the no-op already routed 0.9.0 users to the native class, but it is the next upstream port; the evaluator stays as its staging copy and its unit tests now drive the evaluator directly. Tests: the adapter's unit tests are replaced by TestNativeBnglModel, which pins the ABC seam the exporter relies on (parameter ids and values, entity namespace, formula symbols, state variables) against petab's class and asserts the native loader is present, taking over the retired CI leg's assert step. The corpus validity test passes an explicit model_id because petab derives one from the file stem and requires a PEtab identifier, which the hyphenated corpus filenames are not. tests/_bngl_differential.py carries its own _locate_bng2. The tutorial tests import BnglModel and _locate_bng2 from petab where they still need them. Locally, under petab 0.9.0 on Python 3.12: every test_petab*.py, the PEtab tutorial tests, the lint clinic, the priors lesson, new-era validation, and the packaging metadata tests pass (730 passed). The pinned ruff gate and the -W docs build are clean. ADR-0026 gains an addendum; CHANGELOG records the floor change and the removal.
Builds on #501 (BNGL model support, by @wshlavacek - full credit for the reader implementation and model corpus selection, all of which this branch carries forward unchanged).
.bnglfiles and downloads them on demand